Skip to content

feat: fees on listpeers rpc output #4247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
cdecker merged 3 commits into from
Dec 7, 2020
Merged

feat: fees on listpeers rpc output #4247

cdecker merged 3 commits into from
Dec 7, 2020

Conversation

@m-schmoock
Copy link
Collaborator

@m-schmoock m-schmoock commented Dec 2, 2020 •
edited
Loading

As mentioned in the last developer meeting and additional to #4221 we want to have the local half-channel fees (the ones that are applied when a payment is routed though our node) as part of the listpeers output. Motivation is that this data is non-gossip and currently only available via gossip listchannels.

Example

{
    ...
    "fee_base_msat" : "100msat",
    "fee_proportinal_millionths" : 1000,
    ...
}

This PR adds feature, test and doc.

@m-schmoock m-schmoock requested a review from cdecker as a code owner December 2, 2020 14:31
@m-schmoock m-schmoock mentioned this pull request Dec 2, 2020
Open
ZmnSCPxj
ZmnSCPxj approved these changes Dec 3, 2020
View reviewed changes
Copy link
Contributor

@ZmnSCPxj ZmnSCPxj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 8dbe6f1

Minor quibble about docs but not enough to block this.

@rustyrussell
Copy link
Contributor

rustyrussell commented Dec 4, 2020

OK, so minor feedback:

  1. We prefer to use spec names for fields (unless we've already misuse elsewhere). That makes it fee_base_msat and fee_proportional_millionths.
  2. msat field names should end in _msat so that the Python JSON decoder can automagically turn them into Millisatoshi amounts.

@m-schmoock
feat: adds local halfchan fees to listpeers
13218f6 
This will add `fee_base` (msat) and `fee_ppm` (u32 num) to the RPC
`listpeers` output.

Changelog-Added: fee_base and fee_ppm to listpeers
@m-schmoock
pytest: tests fee_base and fee_ppm on listpeers
5107040
@m-schmoock
doc: fees on listpeers
54af4c9
@m-schmoock
Copy link
Collaborator Author

m-schmoock commented Dec 4, 2020

@rustyrussel
Thx for the feedback. I changed the names.

I figured that in the current listchannels output base_fee_millisatoshi is currently not having a _msat pendant. Should we add this in another PR? If we want to eventually get rid of non-msat fields we need to add this.

@m-schmoock
Copy link
Collaborator Author

m-schmoock commented Dec 4, 2020

@rustyrussell

  1. We prefer to use spec names for fields (unless we've already misuse elsewhere). That makes it fee_base_msat and fee_proportional_millionths.

Note: Just checked, listchannels gives me fee_per_millionth ...

@cdecker
Copy link
Member

cdecker commented Dec 7, 2020

ACK 54af4c9

@cdecker cdecker merged commit 4507203 into ElementsProject:master Dec 7, 2020
@m-schmoock m-schmoock deleted the feat/listpeers_fees branch December 12, 2020 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cdecker cdecker Awaiting requested review from cdecker

1 more reviewer

@ZmnSCPxj ZmnSCPxj ZmnSCPxj approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@m-schmoock @rustyrussell @cdecker @ZmnSCPxj